-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Blocks: Stabilise role
attribute property
#65484
Conversation
const __experimentalHasContentRoleAttribute = jest.fn( () => false ); | ||
const hasContentRoleAttribute = jest.fn( () => false ); | ||
getBlockEditingMode.registry = { | ||
select: jest.fn( () => ( { | ||
__experimentalHasContentRoleAttribute, | ||
hasContentRoleAttribute, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these tests be moved to selectors
tests now?
Size Change: +520 B (+0.03%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! Do you plan to include it in WordPress 6.7?
Looking at the unit tests now |
I think that's a decision that's up for debate. The "Zoom Out mode" feature does make use of contentOnly but not in a way that requires us to know about which blocks are "content blocks". The recent "Select mode" work uses We might want to buy some more time for the API to stablise alongside the new modes, or alternatively we could say the API that's been around for 2yrs is probably good to stablise. Opinions welcomed. |
Anyone have any idea why this test is failing on CI but passing locally? Going to try a rebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So nice to see it ready 👍🏻
The next step is going to be an update to the documentation that explains what the role
property offers so custom blocks can start using it. The related document would be:
https://github.com/WordPress/gutenberg/blob/trunk/docs/reference-guides/block-api/block-attributes.md
There was a conflict while trying to cherry-pick the commit to the wp/6.7 branch. Please resolve the conflict manually and create a PR to the wp/6.7 branch. PRs to wp/6.7 are similar to PRs to trunk, but you should base your PR on the wp/6.7 branch instead of trunk.
|
I am not sure if the Why?
|
We've discussed this above a bit. For me it does make sense.
|
Now that you mentioned it, I checked the resolved discussions and saw it 😓
Of course. I'll create a follow-up PR to remove the alternative suggestion. |
I just noticed this PR didn't cherry pick cleanly. I will do this manually as soon as I can. |
How about updating the block.json schema? Developers will not know what this stabilized field means. |
* Update usages * Update related functions * Remove unwanted alias * Properly deprecate the experimental method * Deprecate and ensure backwards compatibility * Make selector private * Ensure serializer works backwards compat * Ensure experiment API method still exposed on blocks package * Add hint for updating role attribute * Remove unlock * Improve readability of proxying * Add hint to serializer * use stabilised selector for the content role for edit mode * Attempt locking to fix unit test * Ensure mock returns correct value --------- Co-authored-by: getdave <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: ndiego <[email protected]> Co-authored-by: fabiankaegy <[email protected]>
) * Update usages * Update related functions * Remove unwanted alias * Properly deprecate the experimental method * Deprecate and ensure backwards compatibility * Make selector private * Ensure serializer works backwards compat * Ensure experiment API method still exposed on blocks package * Add hint for updating role attribute * Remove unlock * Improve readability of proxying * Add hint to serializer * use stabilised selector for the content role for edit mode * Attempt locking to fix unit test * Ensure mock returns correct value --------- Co-authored-by: Dave Smith <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: ndiego <[email protected]> Co-authored-by: fabiankaegy <[email protected]>
@youknowriad I've drafted a dev note for both stablising the API and also the Dev Note (Draft)As of WordPress 6.7 the This property is part of the Depending on the chosen type, the attribute may then be handled differently at both an API level and/or within the Editor's user interface. The available types are:
Changes to Selectors and API functionsAs part of this effort some selectors and API functions have received changes:
|
role
attribute property
@getdave just as an FYI, I moved this dev note to the miscellaneous editor dev note and will publish it in that :) Thanks for writing it! :) |
What?
Stablises the
_experimentalRole
property of block attributes andallsome related selectors and utils.Note:
__experimentalHasContentRoleAttribute
has been made a private selector. Full backwards compatibility will remain until WP 6.8.Why?
This has been in use for 2 years and has not be stablised. With all the work on
contentOnly
going on it makes sense to look to stablise the APIs in order that block authors can be confident in making use of them.How?
__experimentalHasContentRoleAttribute
a private selector.__experimental
prefixed versions to new versions and throwsdepreciated()
warning__experimentalRole
prop.__experimental
prefixed functions and attribute props.Testing Instructions
hasContentRoleAttribute
API__experimentalGetBlockAttributesNamesByRole
API:The call to the experimental method should throw a deprecated notice.
Also check Editor doesn't crash when moving in / out of Zoom Out mode or the new "Edit" mode.
Registering a Block
Content Only Test Block
to the post and type something into the inputTesting Instructions for Keyboard
Screenshots or screencast